-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stop unwrapping types while mapping #585
Conversation
Oh wait, it ended up being really simple. The question is whether one thinks |
Seems like an improvement to me given it's much more specific than Int. |
Cool. I'll look into that "exhaustive.jl" failure, and if/when fixed we should have TypedSyntax tests passing on 1.10 and 1.11 (but not nightly). Given that inference is allowed to change between releases, I don't think any of these changes count as breaking? (Cthulhu is also the only dependent of TypedSyntax listed on juliahub.) |
Identified in JuliaDebug/Cthulhu.jl#585
Let's see if CodeTracking PR130 fixed the |
TypedSyntax is now passing on 1.11. Since this is a fairly significant change and I'm not 100% confident in our test coverage (as good as it is), I'll play with it interactively a little before merging. |
Modules probably don't need `Core.Const` annotations
This seems to be working reasonably well and without egregious mistakes, so let's merge this. |
This started out as just an attempt to get the remaining tests working on Julia 1.11. However, one of the failures was a case where we wanted to preserve a
Core.Const
wrapper, and as I dug in it became increasingly clear that it was a little risky to alter the type assigned by inference before mapping.This PR preserves (or, intends to preserve) the exact types assigned by inference. It then becomes the
show
component's job to stripCore.Const
and similar wrapper before displaying types.@Zentrik, now the remaining failures seem to be in the vscode portions. Any chance you could take a look at this? I'm less familiar with that code.